Skip to content

Conversation

@cryogenian
Copy link
Member

  • Added functions to provide a way to explicit set decoder for uri (not only decodeURIComponent)
  • Added list combinator that matches provided matcher as argument many times and yields Match (List a)

commit dbc480a
Author: Maxim Zimaliev <zimaliev@yandex.ru>
Date:   Wed Apr 8 18:22:42 2015 +0300

    list combinator

commit 66d9eb9
Author: Maxim Zimaliev <zimaliev@yandex.ru>
Date:   Wed Apr 8 15:05:22 2015 +0300

    added matches' string with explicit decoder

commit 3f0f5ad
Author: Maxim Zimaliev <zimaliev@yandex.ru>
Date:   Wed Apr 8 15:05:00 2015 +0300

    added matches' string with explicit decoder

commit 6460b9f
Merge: aaa82cc 54a5102
Author: Maxim Zimaliev <zimaliev@yandex.ru>
Date:   Wed Apr 8 14:44:28 2015 +0300

    Merge branch 'squashed'

commit 54a5102
Author: Maxim Zimaliev <zimaliev@yandex.ru>
Date:   Wed Apr 8 14:43:35 2015 +0300

    optional decoder for uri support
src/Routing.purs Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Eff feels like it doesn't belong here. You have a few options:

matches :: forall e a b. Match a -> (Maybe a -> a -> b) -> Maybe b

matches :: forall e a b. (Monoid b) => Match a -> (Maybe a -> a -> b) -> b

With the 2nd option, you'd have to submit a PR for purescript-monoid to define Monoid (Eff eff a) when a is a Monoid (which is a sensible monoid to have, just missing).

@cryogenian
Copy link
Member Author

  • removed Debug.Foreign
  • added comment about matches

Don't sure that I understand why there is shouldn't be two Eff in matches. Second argument is effectful callback, that called after hash changed and can be matched with first argument. If previous hash can be matched too, then first argument of callback is Just a else -- Nothing

@jdegoes
Copy link
Contributor

jdegoes commented Apr 8, 2015

The point is that the type signature is extremely specific and can be made more generic. Generic code is easier to get right (there are fewer ways it can go wrong) and is useful solving strictly more problems. The only reason matches / matches' wants to be specific is so it can create an "empty", so there are really two possibilities to make it more generic: require an empty (Monoid) or push that off on the user (return a Maybe).

matches1 :: forall e a b. (Monoid b) => Match a -> (Maybe a -> a -> b) -> b

matches2 :: forall e a b. Match a -> (Maybe a -> a -> b) -> Maybe b

Look at how the old one can be implemented in terms of either of these:

matches3 m f = matches1 m f -- assuming monoid for Eff eff Unit

matches4 m f = fromMaybe (pure unit) (matches2 m f )

In addition to these points, pure code shouldn't require effects when it doesn't have to. It's very surprising to see a matching function expect an effectful callback, it implies the matcher function is going to do something effectful, too, which is not the case.

jdegoes added a commit that referenced this pull request Apr 8, 2015
@jdegoes jdegoes merged commit c8435d1 into purescript-contrib:master Apr 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants